Skip to content

feat: Refactor secret values#3225

Merged
stevehipwell merged 1 commit intointegrations:mainfrom
stevehipwell:refactor-secret-values
Mar 19, 2026
Merged

feat: Refactor secret values#3225
stevehipwell merged 1 commit intointegrations:mainfrom
stevehipwell:refactor-secret-values

Conversation

@stevehipwell
Copy link
Copy Markdown
Collaborator

@stevehipwell stevehipwell commented Feb 25, 2026

Resolves #ISSUE_NUMBER

Before the change?

  • Secret value naming was confusing
  • Encrypted secret value didn't require key_id to be passed in

After the change?

  • Secret value naming is consistent (value & value_encrypted)
  • Secret value names ready for value_wo pattern
  • value_encrypted requires key_id to be passed in

Pull request checklist

  • Schema migrations have been created if needed (example)
  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been reviewed and added / updated if needed (for bug fixes / features)

Does this introduce a breaking change?

Please see our docs on breaking changes to help!

  • Yes
  • No

@stevehipwell stevehipwell added this to the v6.12.0 Release milestone Feb 25, 2026
@stevehipwell stevehipwell self-assigned this Feb 25, 2026
@github-actions
Copy link
Copy Markdown

github-actions Bot commented Feb 25, 2026

👋 Hi! Thank you for this contribution! Just to let you know, our GitHub SDK team does a round of issue and PR reviews twice a week, every Monday and Friday! We have a process in place for prioritizing and responding to your input. Because you are a part of this community please feel free to comment, add to, or pick up any issues/PRs that are labeled with Status: Up for grabs. You & others like you are the reason all of this works! So thank you & happy coding! 🚀

deiga
deiga reviewed Feb 25, 2026
View reviewed changes
Copy link
Copy Markdown
Collaborator

@deiga deiga left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changing the migration test input state doesn't seem correct, could you verify?

Comment thread github/resource_github_actions_organization_secret_migration_test.go Outdated
@stevehipwell stevehipwell force-pushed the refactor-secret-values branch 2 times, most recently from dcb06d3 to e363fd4 Compare February 27, 2026 17:23
@toms-frame toms-frame mentioned this pull request Mar 18, 2026
Open
1 task
@stevehipwell
feat: Refactor secret values
08a6dc7 
Signed-off-by: Steve Hipwell <steve.hipwell@gmail.com>
@stevehipwell stevehipwell force-pushed the refactor-secret-values branch from e363fd4 to 08a6dc7 Compare March 18, 2026 18:12
@stevehipwell stevehipwell marked this pull request as ready for review March 18, 2026 18:14
@stevehipwell
Copy link
Copy Markdown
Collaborator Author

stevehipwell commented Mar 18, 2026

@robert-crandall please could you give this a review?

robert-crandall
robert-crandall reviewed Mar 18, 2026
View reviewed changes
Copy link
Copy Markdown

@robert-crandall robert-crandall left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Before this PR, key_id had ConflictsWith: ["plaintext_value"], meaning key_id + encrypted_value was an explicitly documented (and valid) pattern. The docs said: "This should be provided when setting encrypted_value".

After this PR, key_id gains RequiredWith: ["value_encrypted"]. This means: if key_id is set in config, value_encrypted must also be set. Any user who currently has key_id = "..." alongside encrypted_value = "..." will get a plan-time validation failure immediately on upgrading, despite encrypted_value being nominally "deprecated but still supported."

The fix is either:

  • Remove RequiredWith from key_id (users relying on deprecated encrypted_value + key_id shouldn't be broken), or
  • Add "encrypted_value" to the RequiredWith list: RequiredWith: []string{"value_encrypted", "encrypted_value"} (so both old and new usage pass), or
  • At minimum, document this as a breaking migration requirement with clear upgrade instructions.

@stevehipwell
Copy link
Copy Markdown
Collaborator Author

stevehipwell commented Mar 19, 2026

@robert-crandall the analysis you've got above is muddled, Copilot I assume? I added the key_id and made it conflict with plaintext_value when I fixed the secrets. I wanted to make it required for encrypted_value, as technically it is and without it the value could be un-decryptable, but that would have been a breaking change. This PR add the NEW value_encrypted and made key_id required when this is used. No changes outside of the deprecation for the old fields.

@stevehipwell stevehipwell merged commit 636eff4 into integrations:main Mar 19, 2026
7 checks passed
@stevehipwell stevehipwell deleted the refactor-secret-values branch March 19, 2026 20:10
JiayangZhou pushed a commit to JiayangZhou/terraform-provider-github that referenced this pull request Apr 16, 2026
@stevehipwell @JiayangZhou
feat: Refactor secret values (integrations#3225)
051d1a0 
Signed-off-by: Steve Hipwell <steve.hipwell@gmail.com>
@deiga deiga mentioned this pull request Apr 19, 2026
Closed
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@deiga deiga deiga approved these changes

@robert-crandall robert-crandall robert-crandall approved these changes

Assignees

@stevehipwell stevehipwell

Labels

None yet

Projects

None yet

Milestone

v6.12.0

Development

Successfully merging this pull request may close these issues.

3 participants

@stevehipwell @deiga @robert-crandall